-
Notifications
You must be signed in to change notification settings - Fork 29.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: edited assertion and timeout for test #9889
Conversation
In this change, the setTimeout needed a second argument, so I set that value to 0. In addition, I changed the assertion to be a strictEquals instead of equals.
This was from the Code & Learn at NINA. |
}); | ||
}); | ||
}); | ||
}, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this should be a timer for the purposes of this test, even though it is effectively a setImmediate()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should probably also be a duration of 1
. Duration of 0 gets changed to 1 internally anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that you are at it can you also please change var
occurences to const
and setTimeout
to setImmediate
as suggested by @cjihrig?
Thanks.
@lpinca IAre you sure about the |
@Trott no, I'm not honestly not sure. Happy to keep |
I changed the var declarations to const in this unit test
Changed var declarations. Let me know if there's anything else. |
@danielgsims Thanks! |
Landing. While landing, I'll change the |
In this change, the setTimeout needed a second argument, so I set that value to 1. In addition, I changed the assertion to be a strictEquals instead of equals. I changed the var declarations to const in this test. PR-URL: nodejs#9889 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Landed in b0c10a2. |
In this change, the setTimeout needed a second argument, so I set that value to 1. In addition, I changed the assertion to be a strictEquals instead of equals. I changed the var declarations to const in this test. PR-URL: #9889 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
In this change, the setTimeout needed a second argument, so I set that value to 1. In addition, I changed the assertion to be a strictEquals instead of equals. I changed the var declarations to const in this test. PR-URL: nodejs#9889 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
In this change, the setTimeout needed a second argument, so I set that value to 1. In addition, I changed the assertion to be a strictEquals instead of equals. I changed the var declarations to const in this test. PR-URL: #9889 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
In this change, the setTimeout needed a second argument, so I set that value to 1. In addition, I changed the assertion to be a strictEquals instead of equals. I changed the var declarations to const in this test. PR-URL: #9889 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
In this change, the setTimeout needed a second argument, so I set that value to 1. In addition, I changed the assertion to be a strictEquals instead of equals. I changed the var declarations to const in this test. PR-URL: #9889 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
test
Description of change
In this change, the setTimeout needed a second argument, so I set that
value to 0. In addition, I changed the assertion to be a strictEquals
instead of equals.